Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(WIP) Feat: Sentry Javascript V9 #4568

Open
wants to merge 22 commits into
base: v7
Choose a base branch
from
Open

(WIP) Feat: Sentry Javascript V9 #4568

wants to merge 22 commits into from

Conversation

lucas-zimerman
Copy link
Collaborator

@lucas-zimerman lucas-zimerman commented Feb 19, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

This PR bumps JavaScript to version V9 and also fixes any break changes found.
In short, normal users not experience any changes, as shown on the sample apps, there were no required changes for this bump.

Changes handled:

In regard to shutdownTimeout, we can keep it as an additional parameter or completely remove it if that is the plan.

💡 Motivation and Context

💚 How did you test it?

Sample App, CI, Sentry.io discover

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

  • Update Changelog with breakchanges.
  • Create Migration Doc.

#skip-changelog for now

Copy link
Contributor

github-actions bot commented Feb 19, 2025

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1213.46 ms 1221.42 ms 7.96 ms
Size 2.63 MiB 3.73 MiB 1.10 MiB

Previous results on branch: lz/bump/jsv9

Startup times

Revision Plain With Sentry Diff
71f7309+dirty 1222.02 ms 1224.81 ms 2.79 ms
345e497+dirty 1218.83 ms 1218.73 ms -0.10 ms
e54b1ed+dirty 1227.33 ms 1254.46 ms 27.14 ms
987dc4d+dirty 1216.13 ms 1227.20 ms 11.08 ms

App size

Revision Plain With Sentry Diff
71f7309+dirty 2.63 MiB 3.73 MiB 1.10 MiB
345e497+dirty 2.63 MiB 3.68 MiB 1.05 MiB
e54b1ed+dirty 2.63 MiB 3.68 MiB 1.05 MiB
987dc4d+dirty 2.63 MiB 3.73 MiB 1.10 MiB

Copy link
Contributor

github-actions bot commented Feb 19, 2025

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 383.45 ms 415.65 ms 32.21 ms
Size 17.75 MiB 20.10 MiB 2.36 MiB

Previous results on branch: lz/bump/jsv9

Startup times

Revision Plain With Sentry Diff
71f7309 412.45 ms 431.62 ms 19.18 ms
345e497 397.98 ms 423.40 ms 25.42 ms
e54b1ed 434.79 ms 429.52 ms -5.27 ms
987dc4d 422.12 ms 478.34 ms 56.22 ms

App size

Revision Plain With Sentry Diff
71f7309 17.75 MiB 20.10 MiB 2.36 MiB
345e497 17.75 MiB 20.10 MiB 2.36 MiB
e54b1ed 17.75 MiB 20.10 MiB 2.36 MiB
987dc4d 17.75 MiB 20.10 MiB 2.36 MiB

Copy link
Contributor

github-actions bot commented Feb 19, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 426.84 ms 453.53 ms 26.69 ms
Size 7.15 MiB 8.37 MiB 1.22 MiB

Previous results on branch: lz/bump/jsv9

Startup times

Revision Plain With Sentry Diff
345e497+dirty 375.33 ms 377.23 ms 1.90 ms
71f7309+dirty 361.15 ms 369.90 ms 8.75 ms
e54b1ed+dirty 435.06 ms 429.19 ms -5.87 ms
987dc4d+dirty 411.89 ms 418.60 ms 6.71 ms

App size

Revision Plain With Sentry Diff
345e497+dirty 7.15 MiB 8.37 MiB 1.22 MiB
71f7309+dirty 7.15 MiB 8.37 MiB 1.22 MiB
e54b1ed+dirty 7.15 MiB 8.37 MiB 1.22 MiB
987dc4d+dirty 7.15 MiB 8.37 MiB 1.22 MiB

Copy link
Contributor

github-actions bot commented Feb 19, 2025

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1219.31 ms 1217.37 ms -1.94 ms
Size 3.19 MiB 4.30 MiB 1.11 MiB

Previous results on branch: lz/bump/jsv9

Startup times

Revision Plain With Sentry Diff
71f7309+dirty 1206.00 ms 1214.28 ms 8.28 ms
345e497+dirty 1210.67 ms 1207.50 ms -3.17 ms
e54b1ed+dirty 1227.90 ms 1230.92 ms 3.02 ms
987dc4d+dirty 1209.94 ms 1215.25 ms 5.31 ms

App size

Revision Plain With Sentry Diff
71f7309+dirty 3.19 MiB 4.30 MiB 1.11 MiB
345e497+dirty 3.19 MiB 4.25 MiB 1.06 MiB
e54b1ed+dirty 3.19 MiB 4.25 MiB 1.06 MiB
987dc4d+dirty 3.19 MiB 4.30 MiB 1.11 MiB

@lucas-zimerman
Copy link
Collaborator Author

I will wait for some information in regard to getsentry/sentry-javascript#15471 in case it's a problem on the JavaScript SDK

@krystofwoldrich
Copy link
Member

@lucas-zimerman The yarn.lock seems to be completely regenerated.

How have you upgraded to the JS v9?

We should review it and ensure only the @sentry/* are upgraded to avoid issues like #4592

The yarn.lock change should be readable as only sentry deps should change.

@lucas-zimerman
Copy link
Collaborator Author

@lucas-zimerman The yarn.lock seems to be completely regenerated.

How have you upgraded to the JS v9?

We should review it and ensure only the @sentry/* are upgraded to avoid issues like #4592

The yarn.lock change should be readable as only sentry deps should change.

I fixed on 028dbf9, At the time there were a conflict with yarn.lock, and I created a new one. Next time I will get the target branch yarn.lock and install again on the PR

@krystofwoldrich
Copy link
Member

Thank you @lucas-zimerman, now the yarn.lock looks good.

@lucas-zimerman lucas-zimerman marked this pull request as ready for review February 25, 2025 18:37
@lucas-zimerman
Copy link
Collaborator Author

@antonis @krystofwoldrich I'd say now it's ready for review :D

Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a couple of nits but overall LGTM 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants